Skip to content

novatek: the mask ROM's fallback and UART download, so defib recovers a blank NT9856x - #225

Merged
widgetii merged 2 commits into
masterfrom
novatek-rom-uart
Nov 4, 2026
Merged

widgetii merged 2 commits into
masterfrom
novatek-rom-uart

Conversation

@widgetii

@widgetii widgetii commented Nov 4, 2026

Copy link
Copy Markdown
Member

The NA51089 ROM model now runs the silicon's own recovery when the flash has no valid loader: the fallback order and console letters, and the UART download engine with its quirks. The ROM and its behaviour were read from a dump and the ROM code application note, and checked against an NT98562 and an NT98566.

What defib burn (OpenIPC/defib#169) does against it, unmodified:

  • brings up the real wrloader.bin and the FW98562T.BIN pack over socket://;
  • reaches U-Boot's prompt on nt98562 and nt98566;
  • works both through the straps (boot-strap=0xc) and through the fallback from a blank NOR.

Tests:

  • qemu-boot/test-novatek-recovery.sh runs defib both ways in, and a new CI job runs it.
  • test-novatek.py covers the UART engine and the fallback's letters; all 794 checks pass.

The Ethernet stage (DHCP/TFTP through nvt-eqos) comes in a follow-up PR, after this one merges.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Model NT9856x mask-ROM fallback and UART recovery

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Model NA51089 recovery from invalid flash loaders, including source order, timeouts, and console
 output.
• Implement the mask ROM’s UART download commands so defib can boot a loader from blank flash.
• Add protocol tests and CI recovery runs for NT98562 and NT98566.
Diagram

graph TD
  M["Machine options"] --> R["ROM recovery"] --> F{"Valid flash loader?"} -->|no| SD["SD probe"] --> ETH["Ethernet wait"] --> USB["USB wait"] --> UART["UART commands"] -->|validated| RUN["Loader jump"]
  F -->|yes| RUN
  R -->|UART strap| UART
  R -->|SD strap| SD
  R -->|Ethernet strap| ETH
  R -->|USB strap| USB
  UART -->|rejected or timed out| SD
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Execute a dumped silicon ROM
  • ➕ Could reproduce recovery behavior without hand-modeling individual protocol quirks.
  • ➖ Requires distributing or supplying a ROM image and integrating it with the existing emulated ROM services.

Recommendation: Keep the behavioral model: it works with the existing boot-ROM device and lets tests exercise unmodified defib without a proprietary ROM image. Treat the Ethernet stage’s lack of actual DHCP/TFTP traffic as an explicit scope limit for the follow-up.

Files changed (9) +819 / -10

Enhancement (4) +501 / -6
novatek.cWire recovery controls into NA51089 machine boot +66/-2

Wire recovery controls into NA51089 machine boot

• Adds rom-fallback and rom-fast machine properties and classifies flash versus recovery straps. Instantiates the boot-ROM device for eligible fallback boots even when flash has no loader, while preserving the existing stand-in path otherwise.

qemu/hw/arm/novatek.c

nvt-bootrom.cModel mask-ROM fallback and UART download behavior +407/-4

Model mask-ROM fallback and UART download behavior

• Adds loader validation, strap-selected recovery stages, console output, timers, and a parked CPU that resumes when a loader passes validation. Implements 16-byte UART commands, download storage, checksum and payload quirks, and draining of bytes received outside the UART stage.

qemu/hw/arm/nvt-bootrom.c

novatek.hDeclare machine-to-ROM recovery configuration +6/-0

Declare machine-to-ROM recovery configuration

• Exposes the fallback configuration and flash-strap classification functions used when wiring the Novatek boot-ROM device.

qemu/include/hw/arm/novatek.h

nvt-bootrom.hAdd boot-ROM recovery and UART state +22/-0

Add boot-ROM recovery and UART state

• Extends the device state with recovery options, stage timers, source tracking, and UART command and loader-transfer fields.

qemu/include/hw/arm/nvt-bootrom.h

Documentation (2) +55 / -4
CLAUDE.mdDocument the ROM recovery model in repository guidance +5/-1

Document the ROM recovery model in repository guidance

• Notes when NA51089 fallback runs, identifies the UART recovery path, and links to the recovery test and detailed documentation.

CLAUDE.md

novatek.mdDescribe recovery sources, UART commands, and defib usage +50/-3

Describe recovery sources, UART commands, and defib usage

• Documents strap-selected source order, console output, loader validation, UART protocol quirks, and shortened waits. Adds a blank-flash defib example and clarifies that Ethernet recovery does not yet send DHCP traffic.

docs/novatek.md

Other (3) +263 / -0
ci.ymlRun defib recovery in CI for two NT9856x machines +32/-0

Run defib recovery in CI for two NT9856x machines

• Adds a matrix job for NT98562 and NT98566 that installs defib, downloads the built QEMU binary, and runs both recovery paths.

.github/workflows/ci.yml

test-novatek-recovery.shExercise blank-flash recovery with unmodified defib +95/-0

Exercise blank-flash recovery with unmodified defib

• Starts QEMU with a blank flash for direct UART and NOR-fallback entry, runs defib burn, and verifies that the resulting U-Boot prompt answers a command. Supports SoC selection and retained logs.

qemu-boot/test-novatek-recovery.sh

test-novatek.pyTest UART command quirks and fallback console output +136/-0

Test UART command quirks and fallback console output

• Adds a socket-backed blank-flash fixture and checks command alignment, checksum responses, memory access, payload aliasing, loader execution, and the fallback’s console sequence.

qemu/tests/test-novatek.py

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Invalid flash loaders bypass recovery ✓ Resolved
Description
rom_flash_loader_ok() checks the image checksum but not whether header word 1 matches the SRAM
destination. With a flash strap and fallback enabled, a checksum-valid image naming the wrong
destination skips recovery and enters the direct boot path, even though the fallback loader check
would reject it.

2. Recovery CI tracks whatever defib's main branch holds ✓ Resolved
Description
The boot-novatek-recovery job installs OpenIPC/defib from its mutable default branch rather than a
reviewed commit, then relies on its burn interface and the exact message “U-Boot console ready.”
On push-to-master and pull-request runs, an upstream change or compromise can change the code
executed during installation and the recovery test, while an interface or wording change can fail
the job.


Informational

3. Machine and ROM disagree on flash straps ✓ Resolved
Description
nvt_rom_strap_is_flash counts only straps 0, 2, 3, 6, 8, 9 and 0xA as flash, but
rom_first_source in nvt-bootrom.c (and its comment, "the rest flash") sends every strap other than
0xC, 4, 1, 5 and 0xB to flash. So straps 7, 0xD, 0xE and 0xF switch the fallback on without being
asked, replacing the stand-in on a blank flash, yet the ROM then treats them as flash boots — which
contradicts the docs' "not a flash strap" rule.

4. Snapshots taken during recovery hang 🐞 Bug ☼ Reliability
Description
vmstate_nvt_bootrom saves only args and ret, omitting the fallback stage and retry count, UART
command and transfer progress, and both timers. A snapshot taken while the CPU is parked in WFI
during fallback or a UART download restores without the pending poll or timeout and without the
state needed to finish a partially received command.
Code

qemu/include/hw/arm/nvt-bootrom.h[R49-52]

+    QEMUTimer *timer;           /* the current stage's timeout */
+    QEMUTimer *poll;            /* the UART receive poll */
+    int source;                 /* the stage running, NVT_SRC_* */
+    int tries;
Evidence
The fallback keeps its stage, retry count, UART command and transfer progress in new struct fields
and uses two QEMUTimers for polling and timeouts. The registered device VMState was not extended
beyond args and ret, so it cannot restore the state that drives stage transitions or command
completion.

qemu/include/hw/arm/nvt-bootrom.h[40-59]
qemu/hw/arm/nvt-bootrom.c[970-979]
qemu/include/hw/arm/nvt-bootrom.h[49-59]
qemu/hw/arm/nvt-bootrom.c[728-745]
qemu/hw/arm/nvt-bootrom.c[776-801]
qemu/hw/arm/nvt-bootrom.c[974-984]
qemu/hw/arm/nvt-bootrom.c[1004-1011]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Active fallback stages and UART downloads lose their timers and progress across migration, potentially leaving a restored CPU parked in WFI with no pending wakeup.
## Fix Focus Areas
- qemu/include/hw/arm/nvt-bootrom.h[49-59]
- qemu/hw/arm/nvt-bootrom.c[970-984]
## Recommended Fix
Bump `version_id` and extend the device VMState with `VMSTATE_INT32` for `source`, `tries`, and `nchunk`; `VMSTATE_BOOL` for `storing` and `echo_due`; `VMSTATE_UINT8_ARRAY` for `chunk` and `wr_cdb`; `VMSTATE_UINT32` for `dest`, `size`, and `count`; and `VMSTATE_TIMER_PTR` for `timer` and `poll`. Test migration during a partially received command and during a stage timeout. Alternatively, mark the device unmigratable while `source` is not `NVT_SRC_DONE`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

… a blank NT9856x

nvt-bootrom copied the flash's loader, or a boot-file=, and nothing else:
the ROM's own recovery was not modelled. On NA51089 it now is, as read from
a dump of the ROM and the NT9856x ROM code application note, and checked
against an NT98562 and an NT98566 in the lab:

- rom-fallback=on, or a boot-strap= that is not a flash strap, and a
  flash with no valid loader: the CPU parks in WFI while the ROM tries
  flash, SD, Ethernet, USB and UART, and again from SD, printing the
  silicon's letters (NPIp>K9 S1K2 HE...s100FX S1 ... K: U K:). The
  loader check is the ROM's: word 1 the address, word 9 the size, 0xAA55
  at 0x30, halfwords plus indices summing to 0. Then "T" and the jump.
- The UART stage is the ROM's command engine: 16-byte 0xC0 CDBs, IS_NVT
  (echo and the chip word), OPEN_DEVICE (stops its 30 s timeout),
  ADDR_READ, ADDR_WRITE (acts even with a bad checksum), WRLOADER and
  WAIT_DONE. Its quirks are kept: every 16 bytes are one command, nothing
  resyncs, and the WRLOADER payload is decoded as commands too. Bytes that
  arrive outside the stage are dropped, as a full FIFO drops them.
- Ethernet sends nothing yet: the stage behaves like a network with no
  DHCP server, so the fallback goes on to USB and UART.
- rom-fast=on cuts the DHCP and USB waits tenfold, not the UART window.

OpenIPC's defib (OpenIPC/defib#169) recovers it unmodified: the real
wrloader.bin and FW98562T.BIN from novatek-recovery.tar.gz go in over
socket://, and U-Boot from RAM answers at its prompt, on nt98562 and
nt98566, through the straps (boot-strap=0xc) and through the fallback
from a blank NOR. qemu-boot/test-novatek-recovery.sh does both, and a
new CI job runs it. test-novatek.py gains the engine's cases (stray byte
out of step, bad checksums, a WRLOADER-shaped payload chunk moving the
store, the jump) and the fallback's letters: 794 checks pass.
…raps, defib pinned

Review follow-ups:

- A flash loader whose header names another SRAM base no longer passes
  for valid with the fallback on: rom_flash_loader_ok() checks word 1, as
  the fallback's own check does before the jump.
- nvt_rom_strap_is_flash() follows rom_first_source(): every strap but SD
  (1), Ethernet (4), USB (5, 0xB) and UART (0xC) starts from flash, so a
  reserved strap no longer turns the fallback on.
- CI installs a fixed defib commit, OpenIPC/defib master after #170, not
  whatever its master holds.
@widgetii
widgetii merged commit 8bc72ab into master Nov 4, 2026
43 checks passed
@widgetii
widgetii deleted the novatek-rom-uart branch November 4, 2026 13:53
widgetii added a commit that referenced this pull request Nov 4, 2026
…#226)

* novatek: the mask ROM's Ethernet boot, DHCP and TFTP through nvt-eqos

The fallback's Ethernet stage (#225) sent nothing. It is now the NA51089
ROM's DHCP and TFTP client, as read from a dump of the ROM and seen on an
NT98562 on a real LAN:

- A MAC of 00:00:00 and three random bytes, new every boot.
  DHCPDISCOVER every 5 s.
- The first broadcast offer is taken; a unicast one, to an address it has
  not configured, is dropped, as silicon dropped dnsmasq's.
- DHCPREQUEST, the ACK, an ARP request for the next-server, then one
  TFTP read request for loader.bin from port 2962, to the offer's MAC.
- Then it waits for DATA from that server with no timeout and no
  retransmission, so a network DHCP server that wins the race holds it
  for good. A short block ends it, then the loader check.
- The letters are silicon's: aOfIP <ip> SRV <srv>, MSK, bS2, cAkS3S4,
  CD for ARP, t<block>, Dn, T.

The ROM drives nvt-eqos through a new hook: attached, every frame goes to
it instead of the rings, and it sends on the NIC's queue; it detaches
before the jump, and the loader brings the MAC up as before. Attaching
flushes the queue, or a tap backend that stopped polling while the MAC
could not receive never starts again.

defib's Ethernet recovery (OpenIPC/defib#170, #171) runs against it
unmodified over a tap: the ROM takes defib's offer, fetches loader.bin,
the real Ethernet loader then does its own DHCP and fetches the U-Boot
pack, and U-Boot from RAM answers at its prompt, on nt98562 and nt98566.
qemu-boot/test-novatek-recovery.sh gains that third way in, and CI runs
it (sudo for the tap and ports 67/69), with defib pinned to 74e1bcc.
test-novatek.py drives the ROM over a UDP socket netdev with no root: a
unicast offer dropped, a broadcast one taken, the ARP and the single
read request to the offering MAC, no retransmission while it waits, and
one short block running the loader. 802 checks pass.

* novatek: the DHCPREQUEST names the offer's server, and the test leaves a caller's tap alone

Review follow-ups:

- The ROM's DHCPREQUEST carries the offer's own server identifier
  (option 54) and no longer the next-server, which may be a separate TFTP
  host; the next-server stays the one TFTP is asked.
- test-novatek-recovery.sh changes a pre-existing tap only by adding its
  address if missing, and takes back just that; a tap it made, it
  deletes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant